Skip to content

THRIFT-6017: Upgrade jsdoc to 4.x and override transitive dependency …#3608

Closed
bigjust wants to merge 1 commit into
apache:masterfrom
ibmstorage:THRIFT-6017
Closed

THRIFT-6017: Upgrade jsdoc to 4.x and override transitive dependency …#3608
bigjust wants to merge 1 commit into
apache:masterfrom
ibmstorage:THRIFT-6017

Conversation

@bigjust

@bigjust bigjust commented Jun 26, 2026 •
edited by Jens-G
Loading

Copy link
Copy Markdown
Contributor

…in lib/js

Upgrade jsdoc from ^3.6.7 to ^4.0.0 in lib/js and add an npm override to force grunt-jsdoc to use jsdoc 4.x instead of 3.x. This eliminates taffydb from the lib/js dependency tree.

lib/ts has no jsdoc dependency on current master, so no change needed there.

Client: js

Generated-by: Claude Sonnet 4.6

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

@bigjust @claude
THRIFT-6017: Upgrade jsdoc to 4.x and override transitive dependency …
f293e62 
…in lib/js

Upgrade jsdoc from ^3.6.7 to ^4.0.0 in lib/js and add an npm override
to force grunt-jsdoc to use jsdoc 4.x instead of 3.x. This eliminates
taffydb (CVE-2019-10790) from the lib/js dependency tree.

lib/ts has no jsdoc dependency on current master, so no change needed there.

Client: js

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Generated-by: Claude Sonnet 4.6
Copilot AI review requested due to automatic review settings June 26, 2026 19:50
@bigjust bigjust requested a review from emmenlau as a code owner June 26, 2026 19:50
@mergeable mergeable Bot added the javascript Pull requests that update Javascript code label Jun 26, 2026
Copilot AI reviewed Jun 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the JavaScript library’s documentation tooling to JSDoc 4.x and uses npm overrides to ensure grunt-jsdoc doesn’t pull in JSDoc 3.x, removing taffydb (CVE-2019-10790) from lib/js’s dependency tree.

Changes:

  • Upgrade jsdoc from ^3.6.7 to ^4.0.0 in lib/js.
  • Add an npm overrides entry intended to force grunt-jsdoc to use JSDoc 4.x.
  • Refresh package-lock.json to reflect the new resolved dependency graph (including removal of taffydb).

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/js/package.json Bumps jsdoc to 4.x and adds an npm override to control transitive JSDoc resolution.
lib/js/package-lock.json Updates the lockfile to the new resolved versions, removing taffydb from the tree.
Files not reviewed (1)
  • lib/js/package-lock.json: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/js/package.json
Comment on lines +30 to 32
"overrides": {
"jsdoc": "^4.0.0"
}
@Jens-G

Jens-G commented Jun 30, 2026

Copy link
Copy Markdown
Member

Code review

Found 2 issues (adversarially verified):

  1. The commit body references CVE-2019-10790 directly: "This eliminates taffydb (CVE-2019-10790) from the lib/js dependency tree." AGENTS.md §6 prohibits CVE identifiers and security framing in public-facing text (commit messages, PR descriptions) without qualification — the rule uses "Never" unconditionally. Neutral phrasing: "removes taffydb from the transitive dependency tree."

Verified: CVE reference confirmed in commit body f293e625e; AGENTS.md §6 prohibition confirmed unqualified.

  1. The jsdoc 4.x upgrade is never exercised in CI. lib/js is absent from all 10 CI workflow files — no job runs make -C lib/js check or grunt jsdoc. The lib/js/Gruntfile.js default task includes jsdoc (line 348) and lib/js/Makefile.am check-local runs grunt, but CI only covers lib/nodejs, not lib/js. The upgrade is therefore untested in CI.

thrift/lib/js/Gruntfile.js

Line 348 in f293e62

grunt.registerTask("default", ["test", "concat", "uglify", "jsdoc"]);

Verified: grepped all 10 CI workflow files at PR head; zero references to lib/js, grunt jsdoc, or make -C lib/js.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@Jens-G Jens-G closed this in 073a359 Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@emmenlau emmenlau Awaiting requested review from emmenlau emmenlau is a code owner

Assignees

No one assigned

Labels

javascript Pull requests that update Javascript code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bigjust @Jens-G